feat: expose S3 presigned download URL on FsNode by bigcat88 · Pull Request #419 · cloud-py-api/nc_py_api

bigcat88 · 2026-03-24T09:29:17Z

Add download_url and download_url_expiration properties to FsNodeInfo. The oc:downloadURL property was already requested via PROPFIND but never parsed - now it is. Also request nc:download-url-expiration property.

When the Nextcloud storage backend is S3 with use_presigned_url enabled, these provide a direct download URL bypassing the Nextcloud proxy. Returns empty string / zero when not available (non-S3 backends).

Closes #418

Summary by CodeRabbit

New Features
- File and folder info now include temporary direct-download URLs and associated expiration timestamps so clients can offer time-limited download links and surface availability information to users.
Tests
- Added unit tests verifying defaults, preservation of provided download info, correct parsing of server-provided values, and graceful handling of malformed expiration values.

Add download_url and download_url_expiration properties to FsNodeInfo. The oc:downloadURL property was already requested via PROPFIND but never parsed — now it is. Also request nc:download-url-expiration property. When the Nextcloud storage backend is S3 with use_presigned_url enabled, these provide a direct download URL bypassing the Nextcloud proxy. Returns empty string / zero when not available (non-S3 backends). Closes #418

coderabbitai · 2026-03-24T09:29:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: be778f55-8b74-498c-83ca-a08b36ebe6d8

📥 Commits

Reviewing files that changed from the base of the PR and between e0cf706 and 8ea2767.

📒 Files selected for processing (2)

nc_py_api/files/_files.py
tests_unit/test_download_url.py

📝 Walkthrough

Walkthrough

PROPFIND parsing now captures oc:downloadURL and nc:download-url-expiration; these values are stored on FsNodeInfo and exposed as read-only properties. Unit tests added to validate defaults, parsing, propagation to FsNode, and handling of false/malformed values.

Changes

Cohort / File(s)	Summary
FsNodeInfo data model `nc_py_api/files/__init__.py`	Added `_download_url: str` and `_download_url_expiration: int` to `FsNodeInfo` and new read-only properties `download_url` and `download_url_expiration` (defaults: `""`, `0`).
WebDAV parsing / PROPFIND `nc_py_api/files/_files.py`	Added `nc:download-url-expiration` to PROPFIND properties. `_parse_record` now maps `oc:downloadURL` → `download_url` (ignores empty/`"false"`) and `nc:download-url-expiration` → `download_url_expiration` (int conversion with ValueError/TypeError suppression). Minor string-formatting tweak in exception message.
Tests `tests_unit/test_download_url.py`	New unit tests covering defaults, explicit values, propagation to `FsNode`, parsing when fields present/missing/false, and graceful handling of malformed expiration values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: exposing S3 presigned download URLs on the FsNode public API.
Linked Issues check	✅ Passed	The PR fully addresses issue `#418` by exposing S3 presigned download URL properties on FsNode, enabling direct S3 downloads without Nextcloud proxying.
Out of Scope Changes check	✅ Passed	All changes align with the objective of exposing S3 presigned URLs; no out-of-scope modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/s3-presigned-url

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@nc_py_api/files/__init__.py`:
- Around line 110-111: The constructor assigns self._download_url_expiration
from kwargs without coercion, allowing a string to slip past the intended int
contract; update the code that sets self._download_url_expiration (in the class
__init__ where self._download_url and self._download_url_expiration are
initialized) to coerce the value to an int (e.g.,
int(kwargs.get("download_url_expiration", 0))) and defensively handle
non-numeric inputs (fallback to 0 or raise a clear error) so the attribute is
always an int at runtime.

In `@nc_py_api/files/_files.py`:
- Around line 312-313: The code attempts to parse
prop["nc:download-url-expiration"] into an int unguarded which can raise
ValueError on malformed/non-numeric values; update the block that sets
fs_node_args["download_url_expiration"] to validate or wrap the int(...)
conversion in a try/except ValueError (and TypeError) and on failure assign 0 so
parsing degrades safely; use the existing prop_keys/prop checks and ensure you
reference the same symbols (prop_keys, prop, fs_node_args,
"nc:download-url-expiration") when implementing the guard.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c699e9bc-52be-4512-9abc-efbf722d7ec6

📥 Commits

Reviewing files that changed from the base of the PR and between ef519ae and 6e9c581.

📒 Files selected for processing (2)

nc_py_api/files/__init__.py
nc_py_api/files/_files.py

codecov · 2026-03-24T09:40:27Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.73%. Comparing base (ef519ae) to head (8ea2767).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #419      +/-   ##
==========================================
+ Coverage   94.69%   94.73%   +0.04%     
==========================================
  Files          47       48       +1     
  Lines        5575     5625      +50     
==========================================
+ Hits         5279     5329      +50     
  Misses        296      296

Files with missing lines	Coverage Δ
nc_py_api/files/__init__.py	`99.71% <100.00%> (+<0.01%)`	⬆️
nc_py_api/files/_files.py	`96.53% <100.00%> (+0.12%)`	⬆️
tests_unit/test_download_url.py	`100.00% <100.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add 7 unit tests covering FsNodeInfo defaults, value pass-through, _parse_record with/without presigned URL data, false values from non-S3 backends, and malformed expiration values. Guard int() conversion of download-url-expiration with contextlib.suppress to handle malformed server responses gracefully.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@nc_py_api/files/_files.py`:
- Around line 311-312: The code currently treats any non-empty oc:downloadURL
value as truthy, causing the literal string "false" from xmltodict to be stored
in fs_node_args["download_url"]; update the check around prop_keys/prop for
"oc:downloadURL" (in _files.py) to only assign fs_node_args["download_url"] when
prop["oc:downloadURL"] is neither empty nor the literal string "false"
(case-insensitive), otherwise leave/set the default empty string to conform to
the FSNode API.

In `@tests_unit/test_download_url.py`:
- Around line 59-70: The fixture in test_parse_record_with_false_download_url
uses boolean False for "oc:downloadURL" and "nc:download-url-expiration" but
real payloads contain the string "false"; update the values in the test fixture
inside the test_parse_record_with_false_download_url function to use the string
"false" for both "oc:downloadURL" and "nc:download-url-expiration" so the test
mirrors actual XML text parsing and exercises the intended regression path.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9de4e952-0337-4db5-a0df-33a79cc19d28

📥 Commits

Reviewing files that changed from the base of the PR and between 6e9c581 and e0cf706.

📒 Files selected for processing (2)

nc_py_api/files/_files.py
tests_unit/test_download_url.py

When S3 storage doesn't support presigned URLs, the server returns <oc:downloadURL>false</oc:downloadURL> which xmltodict parses as the string "false" (truthy in Python). Filter it out explicitly. Also update the test fixture to use string "false" instead of boolean False to match real xmltodict behavior.

coderabbitai Bot reviewed Mar 24, 2026

View reviewed changes

Comment thread nc_py_api/files/__init__.py

Comment thread nc_py_api/files/_files.py Outdated

coderabbitai Bot reviewed Mar 24, 2026

View reviewed changes

Comment thread nc_py_api/files/_files.py Outdated

Comment thread tests_unit/test_download_url.py

oleksandr-nc approved these changes Mar 25, 2026

View reviewed changes

oleksandr-nc merged commit a0f8a31 into main Mar 25, 2026
15 checks passed

oleksandr-nc deleted the feat/s3-presigned-url branch March 25, 2026 07:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: expose S3 presigned download URL on FsNode#419

feat: expose S3 presigned download URL on FsNode#419
oleksandr-nc merged 3 commits into
mainfrom
feat/s3-presigned-url

bigcat88 commented Mar 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 24, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bigcat88 commented Mar 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bigcat88 commented Mar 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 24, 2026 •

edited

Loading

codecov Bot commented Mar 24, 2026 •

edited

Loading